Skip to content

Conversation

@ViktorHofer
Copy link
Member

Failed in the last few rolling builds at least: #18090.

cc @dotnet/ncl

@ViktorHofer ViktorHofer requested a review from safern February 12, 2020 15:03
@ViktorHofer ViktorHofer self-assigned this Feb 12, 2020
@stephentoub stephentoub merged commit 581d298 into master Feb 12, 2020
@stephentoub stephentoub deleted the ViktorHofer-patch-2 branch February 12, 2020 17:04
Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


[Fact]
[PlatformSpecific(TestPlatforms.Linux)] // Some APIs are not supported on Linux
[ActiveIssue(18090)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I believe we should be putting the full URL, @stephentoub changed all previous issues to do that as you can just CTRL+Click from the IDE.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we then remove the int constructor from ActiveIssueAttribute?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's a good idea. Are any other repos using it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I was going to do that and completely forgot about it. Will put up a PR now.

@davidsh davidsh added this to the 5.0 milestone Feb 12, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants